-
Notifications
You must be signed in to change notification settings - Fork 682
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SOLR-17589 First use of PUT/POST request with HttpJdkSolrClient generates error log entry on solr server due to initial HEAD request #2926
Conversation
This is good. Is there any possibility you can add an assert somewhere to |
Thanks for the encouragement. |
Here is an approach that won't need any change to the client itself.
|
…StatusCode == 200)" This reverts commit 692c709.
The initial problem was solved by adding the http header "Content-Type" while keeping the noBody() (see below), so no need to store the body in a separate variable.
I reverted the change and test on the http status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting unit test for something hard to actually test!
@Paul-Blanchaert This is great, although I am surprised the host jetty behaves such. Can you both sync this branch to Once the CHANGES.txt entry is there, we can go ahead and merge & backport for 9.9. |
@jdyer1 |
…ates error log entry on solr server due to initial HEAD request (#2926)
https://issues.apache.org/jira/browse/SOLR-17589
Description
Adding header "Content-Type"="application/json" to HEAD request prevents that the server side raises the SolrException.
Solution
Added header "Content-Type"="application/json" to HEAD request
Tests
Existing unit tests show the issue in the test log (and ignore the server side SolrException).
Re-running the tests with this commit don't show the issue.
Adding a new integration test (client side changes raises server side exception) seems too cumbersome for this change where positive result is demonstrated in the test log and sufficient existing unit test validate that the functionality works fine. Furthermore, it seems to me that adding a (formerly missing) valid http header to a client side http request should not raise issues that are not covered by current test set. When that specific header fails, it should fail fast.
Checklist
Please review the following and check all that apply:
main
branch../gradlew check
(with -Pvalidation.rat.failOnError=false).